libvpx: fix configuration for arm64 #9252

jpanetta · 2020-11-26T23:05:16Z

Disable switch to the iphoneos sdk for arm targets
Add arm64 to supported_archs

Fixes: https://trac.macports.org/ticket/60940

Description

Gets libvpx building on Apple Silicon macs.

Type(s)

bugfix
enhancement
security fix

Tested on

macOS 11.0.1
Xcode 12.2

Verification

Have you

followed our Commit Message Guidelines?
squashed and minimized your commits?
checked that there aren't other open pull requests for the same change?
referenced existing tickets on Trac with full URL?
checked your Portfile with port lint?
tried a full install with sudo port -vst install?
tested basic functionality of all binary files?

macportsbot · 2020-11-26T23:05:19Z

Notifying maintainers:
@dbevans for port libvpx.

macportsbot · 2020-11-27T00:33:37Z

Travis Build #15413 Passed.

Lint results

--->  Verifying Portfile for libvpx
--->  0 errors and 0 warnings found.

Port libvpx success on xcode10.3. Log
Port libvpx success on xcode9.4. Log
Port libvpx success on xcode8.3. Log
Port libvpx success on xcode7.3. Log

ryandesign · 2020-11-27T04:39:02Z

multimedia/libvpx/Portfile

@@ -72,6 +72,10 @@ configure.args      --enable-vp8 \
                    --disable-examples \
                    --disable-unit-tests

+if { ${build_arch} eq "arm64" } {


This will not give you a correct build when using the universal variant.

Thanks for catching this! Hopefully the update I just pushed is the right way to fix this.

macportsbot · 2020-11-27T07:22:54Z

Travis Build #15419 Passed.

Lint results

--->  Verifying Portfile for libvpx
--->  0 errors and 0 warnings found.

Port libvpx success on xcode10.3. Log
Port libvpx success on xcode9.4. Log
Port libvpx success on xcode8.3. Log

ryandesign · 2020-11-28T11:55:52Z

multimedia/libvpx/files/patch-build-make-configure.sh.diff

+   # Handle darwin variants. Newer SDKs allow targeting older
+   # platforms, so use the newest one available.
+   case ${toolchain} in
+-    arm*-darwin*)


Your patch removes the arm*-darwin* case because it was iOS specific, but there's still an x86*-darwin* case below. Are we sure that whatever the x86*-darwin* case is doing for x86 macOS isn't also needed for arm64 macOS?

The x86*-darwin* version was setting -isysroot to the MacOSX SDK, which wasn't needed for my build, but indeed looks important. I've fixed this now.

BTW, I just noticed #59360, which now is triggered on arm by including the x86*-darwin* lines.

That entire case block with the nonsense osx_sdk_dir="$(configure.sdkroot)" line has no effect apart from emitting a non-fatal command not found error.

So the correct -isysroot cflags and -syslibroot ldflags were actually being set on my system already without this block--though I'm not sure exactly how. It looks like we should just remove this block entirely.

multimedia/libvpx/Portfile

macportsbot · 2020-11-29T03:27:35Z

Travis Build #15464 Passed.

Lint results

--->  Verifying Portfile for libvpx
--->  0 errors and 0 warnings found.

Port libvpx success on xcode10.3. Log
Port libvpx success on xcode9.4. Log
Port libvpx success on xcode8.3. Log

kencu · 2020-12-20T15:19:04Z

NB: upstream fix is here:

webmproject/libvpx@979e27c

kencu · 2020-12-20T15:22:37Z

and this is helpful too: webmproject/libvpx@723fca7

kencu · 2020-12-20T23:27:36Z

I did this, and it worked today:

diff --git a/opt/macportsofficial/macports-ports/multimedia/libvpx/Portfile b/Portfile
index 5b97b55..81c6fea 100644
--- a/opt/macportsofficial/macports-ports/multimedia/libvpx/Portfile
+++ b/Portfile
@@ -31,13 +31,18 @@ fetch.type          git
 git.url             https://chromium.googlesource.com/webm/${name}
 git.branch          v${version}
 
+platform arm {
+# use the latest 20201215 commit
+git.branch          8ed23d5f7fd1a003ecd1eb543f3a662772306155
+}
+
+
 # support for non-intel archs removed in version 1.5.0
-supported_archs     x86_64 i386
+supported_archs     x86_64 i386 arm64
 
 depends_build-append port:yasm
 
-patchfiles          patch-build-make-configure.sh.diff \
-                    patch-Makefile.diff
+patchfiles           patch-Makefile.diff
 
 # clang 425.0.28 with libvpx-1.4.0:
 # vp9/common/x86/vp9_subpixel_8t_intrin_avx2.c:77:16: error: assigning to '__m256i' from incompatible type 'int'
@@ -64,7 +69,6 @@ configure.args      --enable-vp8 \
                     --enable-pic \
                     --enable-postproc \
                     --enable-multithread \
-                    --enable-runtime-cpu-detect \
                     --enable-experimental \
                     --enable-shared \
                     --disable-install-docs \
@@ -72,6 +76,12 @@ configure.args      --enable-vp8 \
                     --disable-examples \
                     --disable-unit-tests
 
+# for intel systems, enable runtime cpu detection features
+# for arm64 on darwin, don't as we only have the M1 and this fails anyway
+if {${os.platform} in "i386 x86_64"} {
+    configure.args-append --enable-runtime-cpu-detect
+}
+
 configure.env       LD=${configure.cc}
 
 # add in when docs are installed correctly
@@ -105,13 +115,14 @@ configure.universal_args-delete --disable-dependency-tracking
 
 set my_targets(i386)    x86
 set my_targets(x86_64)  x86_64
+set my_targets(arm64)     arm64
 
 # We must specify the target, otherwise the configure script will guess,
 # and that may not match what the user requested in macports.conf.
-foreach my_arch {i386 x86_64} {
+foreach my_arch {i386 x86_64 arm64} {
     set merger_host(${my_arch}) ""
     if {[info exists my_targets(${my_arch})]} {
-        set merger_configure_args(${my_arch}) --force-target=$my_targets(${my_arch})-${os.platform}-gcc
+        set merger_configure_args(${my_arch}) --force-target=$my_targets(${my_arch})-${os.platform}${os.major}-gcc
     }
 }
 if {![variant_isset universal]} {

dw808303 · 2021-01-08T01:29:05Z

Is this one going to get checked in?

mascguy · 2021-01-23T01:13:47Z

Folks, since ffmpeg depends on libvpx, plenty of downstream ports are being blocked for Big Sur ARM64...

mascguy · 2021-01-23T01:29:01Z

I did this, and it worked today:

Ken, are your changes release-worthy, such that we could push this PR through...?

kencu · 2021-01-23T08:49:00Z

kinda hack.

no doubt Ryan has a much better plan, or we can wait for a new release upstream, which for all I know, not having recently looked, is already released...

ryandesign · 2021-01-23T08:58:36Z

+# for intel systems, enable runtime cpu detection features
+# for arm64 on darwin, don't as we only have the M1 and this fails anyway
+if {${os.platform} in "i386 x86_64"} {
+    configure.args-append --enable-runtime-cpu-detect
+}

os.platform will never have the value x86_64. It will only ever have the value powerpc, i386 or arm.

Basing this decision on os.platform isn't right, since on either arm or i386 platform you might be building universal for both arm64 and x86_64, and you will want runtime CPU detection to be enabled for the x86_64 part and disabled for the arm64 part. Using merger_configure_args is correct.

mascguy · 2021-01-24T01:38:35Z

While I don't have an M1 Mac (or a Big Sur VM yet either, for that matter), I can certainly try cross-compilation on a friend's Intel-based machine.

If that would help move this along... ?

kencu · 2021-01-24T01:46:01Z

yeah, I copy/pasted that bit out of this PR due to laziness, but it should be

+if {${configure.build_arch} in "i386 x86_64"} {
+    configure.args-append --enable-runtime-cpu-detect
+}

this stuff: merger_configure_args always makes me go back and read the muniversal PG code again to figure out WTH is going on with it.

kencu · 2021-01-24T01:47:56Z

You can't really do this kind of fix without an arm mac to see if it actually works (and passes the tests, ideally)...

Without a test run, it's all just imaginary.

mascguy · 2021-01-24T01:52:55Z

You can't really do this kind of fix without an arm mac to see if it actually works (and passes the tests, ideally)...

Without a test run, it's all just imaginary.

True. But was hoping to help with any related minutia, to simply keep things moving.

ryandesign · 2021-01-25T15:36:10Z

yeah, I copy/pasted that bit out of this PR due to laziness, but it should be
+if {${configure.build_arch} in "i386 x86_64"} {
+    configure.args-append --enable-runtime-cpu-detect
+}

No, it shouldn't. Set it properly in the correct array elements(s) of merger_configure_args. The port already has code to add the correct array element from merger_configure_args when not building universal so that's already taken care of.

kencu · 2021-01-25T16:49:44Z

ryan, if you want to fix up this PR I'll test it.

hexagonal-sun · 2021-01-27T16:46:45Z

After applying:

diff --git a/multimedia/libvpx/Portfile b/multimedia/libvpx/Portfile
index 4b1cc24f986..dee7784e2fb 100644
--- a/multimedia/libvpx/Portfile
+++ b/multimedia/libvpx/Portfile
@@ -64,7 +64,6 @@ configure.args      --enable-vp8 \
                     --enable-pic \
                     --enable-postproc \
                     --enable-multithread \
-                    --enable-runtime-cpu-detect \
                     --enable-experimental \
                     --enable-shared \
                     --disable-install-docs \
@@ -76,8 +75,8 @@ configure.args      --enable-vp8 \
 # For arm64 builds (and universal builds targeting arm) we disable this feature,
 # meaning NEON support is determined at compile time.
 # For x86* architectures in universal builds, this feature is re-enabled below.
-if { ${build_arch} eq "arm64" || [variant_isset universal] } {
-    configure.args-delete --enable-runtime-cpu-detect
+if {${configure.build_arch} in "i386 x86_64"} {
+    configure.args-append --enable-runtime-cpu-detect
 }

I can confirm this builds on an arm64 mac.

If this could get merged soon, that would be great!

kencu · 2021-01-27T16:54:06Z

It's still not quite right, but it does build.

@ryandesign has pointed out what needs to be done, but he has 15 years more experience than I do mucking around with gorey dets of the muniversal PortGroup, and I just have not had time to read through that portgroup's sourcefiles and fix it the way he wants (the right way).

So -- it sits until someone fixes it properly. This PR is not right, and should not be committed as is.

mascguy · 2021-02-01T15:31:53Z

Sorry to be a pest. But in the immortal words of fictional character Monica Geller, from the TV show Friends: "Nagging Works!" :-)

Jokes aside, we really need to get this port building on arm64. So... what can I do to help?

codesmythe · 2021-02-02T16:26:01Z

I think this diff against this PR's Portfile would do the job. It

removes the --enable-runtime-cpu-detect from the starting list of configure options.
Removes the part that conditionally deletes it. As @ryandesign points out, the Portfile already has code to add --enable-runtime-cpu-detect back on the configure options list for architectures that support it.

--- multimedia/libvpx/Portfile.orig     2021-02-02 09:14:01.000000000 -0600
+++ multimedia/libvpx/Portfile  2021-02-02 09:14:25.000000000 -0600
@@ -64,7 +64,6 @@
                     --enable-pic \
                     --enable-postproc \
                     --enable-multithread \
-                    --enable-runtime-cpu-detect \
                     --enable-experimental \
                     --enable-shared \
                     --disable-install-docs \
@@ -72,14 +71,6 @@
                     --disable-examples \
                     --disable-unit-tests

-# libvpx does not yet support runtime detection of NEON features on macOS/iOS.
-# For arm64 builds (and universal builds targeting arm) we disable this feature,
-# meaning NEON support is determined at compile time.
-# For x86* architectures in universal builds, this feature is re-enabled below.
-if { ${build_arch} eq "arm64" || [variant_isset universal] } {
-    configure.args-delete --enable-runtime-cpu-detect
-}
-
 configure.env       LD=${configure.cc}

 # add in when docs are installed correctly

Resulting configure line for arm64, show absence of --enable-runtime-cpu-detect

Executing:  cd "/opt/local/var/macports/build/_Users_robg_macports-ports_multimedia_libvpx/libvpx/work/libvpx-1.9.0" && ./configure --prefix=/opt/local --enable-vp8 --enable-vp9 --enable-internal-stats --enable-pic --enable-postproc --enable-multithread --enable-experimental --enable-shared --disable-install-docs --disable-debug-libs --disable-examples --disable-unit-tests --force-target=arm-darwin-gcc

Resulting configure line for x86_64, showing --enable-runtime-cpu-detect at the end of the line.

Executing:  cd "/opt/local/var/macports/build/_Users_robg_macports-ports_multimedia_libvpx/libvpx/work/libvpx-1.9.0" && ./configure --prefix=/opt/local --enable-vp8 --enable-vp9 --enable-internal-stats --enable-pic --enable-postproc --enable-multithread --enable-experimental --enable-shared --disable-install-docs --disable-debug-libs --disable-examples --disable-unit-tests --force-target=x86_64-darwin-gcc --enable-runtime-cpu-detect

kencu · 2021-02-02T16:31:00Z

I had to use a newer upstream github commit, not yet released, to build libvpx a month ago, as above, on arm64.

codesmythe · 2021-02-02T16:41:27Z

The commit you referenced deals with changing configure to deal with arm-ios vs. arm-mac. This PR also patches configure for that reason but in a different way. So should this PR then be reworked to use a patch based on the upstream one? The current PR with my additional diff does build on arm64 and x86_64 (Big Sur) for me.

kencu · 2021-02-02T17:12:53Z

The diff I put up a month ago built libvpx on BigSur arm64 and Intel as well, for me and anyone else who used it.

We would always use the upstream patch, if available. In this case, there were a bunch of fixes, so I used a newer upstream commit.

The only remaining issue we have is finessing the Portfile into using the muniversal PortGroup official commands, as Ryan pointed out.

* Disable switch to the iphoneos sdk for arm targets * Disable unsupported "runtime-cpu-detect" feature on arm64 * Add arm64 to `supported_archs` Fixes: https://trac.macports.org/ticket/60940

kencu · 2021-02-04T06:09:56Z

OK -- I spent some time sorting this out, I think, after dinner here. I looks like this does all the right things on the BigSur Intel system I am presently working on.

Need to test a pure arm64 build, a universal arm64 build, and then some universal i386/x86_64 builds to make sure this is covering all the bases properly.

multimedia/libvpx/Portfile

kencu · 2021-02-04T18:34:07Z

Catalina:
works right on Catalina, installs v 1.9.0, same as previous. No universal variant available there.

hexagonal-sun · 2021-02-04T19:58:35Z

@kencu I've tested this on my 64-bit Mac this end. All looks good.

Pure arm64 build:

matthew@matthews-air ~/D/ports> sudo port install libvpx
--->  Computing dependencies for libvpx
--->  Fetching distfiles for libvpx
--->  Verifying checksums for libvpx
--->  Extracting libvpx
--->  Applying patches to libvpx
--->  Configuring libvpx
--->  Building libvpx
--->  Staging libvpx into destroot
--->  Installing libvpx @1.9.0-20210203_0
--->  Activating libvpx @1.9.0-20210203_0
--->  Cleaning libvpx
--->  Scanning binaries for linking errors
--->  No broken files found.
matthew@matthews-air ~/D/ports > lipo -archs  /opt/local/lib/libvpx.6.dylib
arm64

Universal build

matthew@matthews-air ~/D/ports> sudo port install libvpx +universal
--->  Computing dependencies for libvpx
--->  Building libvpx
--->  Staging libvpx into destroot
--->  Installing libvpx @1.9.0-20210203_0+universal
--->  Deactivating libvpx @1.9.0-20210203_0
--->  Cleaning libvpx
--->  Activating libvpx @1.9.0-20210203_0+universal
--->  Cleaning libvpx
--->  Scanning binaries for linking errors
--->  No broken files found.
--->  No broken ports found.
matthew@matthews-air ~/D/ports> lipo -archs  /opt/local/lib/libvpx.6.dylib
x86_64 arm64

FYI I'm running BigSur (11.2) and have tested basic functionalty with mpv.

Hope that helps!

kencu · 2021-02-04T20:52:57Z

Very good, thanks. Plain and universal builds on 10.6 and 10.11 work properly as well.

So that is looking like a go.

Last thing is to just nail down that version thing I did, make sure that is the way to go.

@ryandesign @dbevans -- looking to commit this shortly, speak now or forever hold your peace please.

rework universal building to current MP standards

macportsbot assigned dbevans Nov 26, 2020

macportsbot added maintainer: open Affects an openmaintainer port type: bugfix labels Nov 26, 2020

ryandesign reviewed Nov 27, 2020

View reviewed changes

jpanetta force-pushed the libvpx_arm64 branch from a0d22b6 to 9e1e8f7 Compare November 27, 2020 07:03

ryandesign reviewed Nov 28, 2020

View reviewed changes

multimedia/libvpx/Portfile Show resolved Hide resolved

jpanetta force-pushed the libvpx_arm64 branch 2 times, most recently from f29ef98 to ac7aa72 Compare November 29, 2020 00:07

sirn mentioned this pull request Dec 4, 2020

mpv: fix building on apple silicon #9337

Merged

9 tasks

libvpx: fix configuration for arm64

d78818b

* Disable switch to the iphoneos sdk for arm targets * Disable unsupported "runtime-cpu-detect" feature on arm64 * Add arm64 to `supported_archs` Fixes: https://trac.macports.org/ticket/60940

kencu force-pushed the libvpx_arm64 branch from ac7aa72 to d78818b Compare February 4, 2021 02:07

kencu force-pushed the libvpx_arm64 branch from c9c3c8a to ded1440 Compare February 4, 2021 06:16

kencu reviewed Feb 4, 2021

View reviewed changes

multimedia/libvpx/Portfile Show resolved Hide resolved

libvpx: use newer commit to support arm64

66dece4

rework universal building to current MP standards

kencu force-pushed the libvpx_arm64 branch from ded1440 to 66dece4 Compare February 5, 2021 01:44

kencu merged commit f0c882c into macports:master Feb 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libvpx: fix configuration for arm64 #9252

libvpx: fix configuration for arm64 #9252

jpanetta commented Nov 26, 2020

macportsbot commented Nov 26, 2020

macportsbot commented Nov 27, 2020

ryandesign Nov 27, 2020

jpanetta Nov 27, 2020

macportsbot commented Nov 27, 2020

ryandesign Nov 28, 2020

jpanetta Nov 28, 2020

jpanetta Nov 29, 2020 •

edited

macportsbot commented Nov 29, 2020

kencu commented Dec 20, 2020

kencu commented Dec 20, 2020

kencu commented Dec 20, 2020

dw808303 commented Jan 8, 2021

mascguy commented Jan 23, 2021

mascguy commented Jan 23, 2021

kencu commented Jan 23, 2021

ryandesign commented Jan 23, 2021

mascguy commented Jan 24, 2021

kencu commented Jan 24, 2021 •

edited

kencu commented Jan 24, 2021

mascguy commented Jan 24, 2021

ryandesign commented Jan 25, 2021

kencu commented Jan 25, 2021

hexagonal-sun commented Jan 27, 2021

kencu commented Jan 27, 2021

mascguy commented Feb 1, 2021

codesmythe commented Feb 2, 2021 •

edited

kencu commented Feb 2, 2021 •

edited

codesmythe commented Feb 2, 2021

kencu commented Feb 2, 2021

kencu commented Feb 4, 2021

kencu commented Feb 4, 2021

hexagonal-sun commented Feb 4, 2021 •

edited

kencu commented Feb 4, 2021

libvpx: fix configuration for arm64 #9252

libvpx: fix configuration for arm64 #9252

Conversation

jpanetta commented Nov 26, 2020

Description

Type(s)

Tested on

Verification

macportsbot commented Nov 26, 2020

macportsbot commented Nov 27, 2020

ryandesign Nov 27, 2020

Choose a reason for hiding this comment

jpanetta Nov 27, 2020

Choose a reason for hiding this comment

macportsbot commented Nov 27, 2020

ryandesign Nov 28, 2020

Choose a reason for hiding this comment

jpanetta Nov 28, 2020

Choose a reason for hiding this comment

jpanetta Nov 29, 2020 • edited

Choose a reason for hiding this comment

macportsbot commented Nov 29, 2020

kencu commented Dec 20, 2020

kencu commented Dec 20, 2020

kencu commented Dec 20, 2020

dw808303 commented Jan 8, 2021

mascguy commented Jan 23, 2021

mascguy commented Jan 23, 2021

kencu commented Jan 23, 2021

ryandesign commented Jan 23, 2021

mascguy commented Jan 24, 2021

kencu commented Jan 24, 2021 • edited

kencu commented Jan 24, 2021

mascguy commented Jan 24, 2021

ryandesign commented Jan 25, 2021

kencu commented Jan 25, 2021

hexagonal-sun commented Jan 27, 2021

kencu commented Jan 27, 2021

mascguy commented Feb 1, 2021

codesmythe commented Feb 2, 2021 • edited

kencu commented Feb 2, 2021 • edited

codesmythe commented Feb 2, 2021

kencu commented Feb 2, 2021

kencu commented Feb 4, 2021

kencu commented Feb 4, 2021

hexagonal-sun commented Feb 4, 2021 • edited

Pure arm64 build:

Universal build

kencu commented Feb 4, 2021

jpanetta Nov 29, 2020 •

edited

kencu commented Jan 24, 2021 •

edited

codesmythe commented Feb 2, 2021 •

edited

kencu commented Feb 2, 2021 •

edited

hexagonal-sun commented Feb 4, 2021 •

edited